Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dagger Instance #147

Closed
wants to merge 13 commits into from
Closed

Conversation

jpfairbanks
Copy link
Member

This PR starts work on parallel computing with Catlab by making an instance of GLA for Dagger DAGs. This allows us to execute linear maps in parallel using the Dagger scheduler.

experiments/DaggerInstanceGLA/Manifest.toml Outdated Show resolved Hide resolved
experiments/DaggerInstanceGLA/src/DaggerInstanceGLA.jl Outdated Show resolved Hide resolved
experiments/DaggerInstanceGLA/src/DaggerInstanceGLA.jl Outdated Show resolved Hide resolved
experiments/DaggerInstanceGLA/src/DaggerInstanceGLA.jl Outdated Show resolved Hide resolved
braid(V::DagDom, W::DagDom) =
MatrixThunk(delayed(x->x)(LinearMap(braid_lm(V.N), braid_lm(W.N), W.N+V.N, V.N+W.N)),V.N+W.N, W.N+V.N)

mcopy(V::DagDom) = MatrixThunk(delayed(x->x)(LinearMap(mcopy_lm, plus_lm, 2*V.N, V.N)), V.N, 2*V.N)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these should be MatrixThunk(delayed(x->LinearMap(mcopy_lm, plus_lm, 2*V.N, V.N)*x), V.N, 2*V.N) etc.

Copy link
Member

@bosonbaas bosonbaas Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I see what you're getting at with that, I had a hard time with this one (as well as braid and delete). I think as-is will cause a typeError, since delayed(x->LinearMap(mcopy_lm, plus_lm, 2*V.N, V.N)*x) is not of type Thunk.

From the way it's used in the LinearMaps instance, mcopy returns a morphism (LinearMap object) which is then applied to other morphisms through composition. If we're keeping with that usage (applying mcopy by composition), I think we'll want this to return a MatrixThunk which has an identity Thunk for the LinearMap mcopy, which can then be used as an argument in a compose call.

This acts to copy the internal LinearMap through composition.

experiments/DaggerInstanceGLA/src/test.jl Outdated Show resolved Hide resolved
@jpfairbanks
Copy link
Member Author

@bosonbaas, I think this is a good start. I noted some stuff that can be simplified. And we need some tests before this is ready to merge.

@bosonbaas
Copy link
Member

Thanks for the comments! It might be useful to wait on pulling this request until we transfer Catlab.Experiments over to it's own dedicated folder ( #144 ) so that this can be placed into that structure.

@epatters
Copy link
Member

epatters commented Mar 27, 2020

Very cool. Dagger is new to me, although I once used dask, which looks similar.

Thanks for getting this started, Andrew. I agree that resolving #144 first makes sense.

@bosonbaas
Copy link
Member

Currently the only tests I have are comparing the results of expressions evaluated as LinearMaps and as MatrixThunks in an attempt to show equivalence between the two instances. What other tests would be good to have in here?
I'll hold off on asking about the structure of the tests directory here, as I'll put that in #144 for discussion.

@jpfairbanks
Copy link
Member Author

Here are the notes from our conversation today, I think we might want to have both the OpenDAG approach you implemented here and a "CT-native" approach like the following Thunk implementation.

AbstractThunk

struct ThunkDom
  types::Vector{DataType}
end
  
struct GeneratorThunk <: AbstractThunk
  dom
  codom
  t::Thunk
end

CompositeThunk <: AbstractThunk
  fs::Vector{AbstractThunk}
end
  
ProductThunk <: AbstractThunk
  fs::Vector{AbstractThunk}
end
  
CopyThunk <: AbstractThunk
  x::ThunkDom
end

BraidThunk <: AbstractThunk
  x::ThunkDom
  y::ThunkDom
end

# copy(X::Ob) → (x)↦CopyThunk(x,y)
# braid(X::Ob, Y::Ob) → (x,y)↦BraidThunk(x,y)

# f⊗g⋅h⋅Δ(x)... ↦ CompositeThunk(ProductThunk(f,g), h, CopyThunk(x))

@epatters
Copy link
Member

epatters commented Apr 9, 2020

I haven't looked at this carefully yet, but is the comment I made in #157 relevant here as well?

@jpfairbanks
Copy link
Member Author

Yes it is.

@epatters epatters marked this pull request as draft September 12, 2020 22:58
@epatters
Copy link
Member

epatters commented Nov 21, 2020

@jpfairbanks and @bosonbaas, what are your plans for this experiment? Looks like a solid amount of work has gone into it. One possibility is to just merge it into the experiments basically as is. For that, we would just need to make sure that all the tests still pass and update the experiments GitHub CI action to run the tests (otherwise, bit rot is inevitable).

@jpfairbanks
Copy link
Member Author

I think we would want to update it to use something like oapply(d::DWD, args::Vector{Thunk})::Thunk then you would get that Dagger Thunks are an operad algebra for DWDs.

@bosonbaas
Copy link
Member

I've made an evaluate function in Algebraic Relations PR 17 which implements the conversion from DWD to Thunk and then evaluates that Thunk.
Would this make sense as something to put in the experiments folder (after being converted to simply an oapply for Thunks)?

@jpfairbanks
Copy link
Member Author

I think it makes sense to pull the workflow parts of algebraic relations out into a new package and put this in that package as a DAG construction tool. Algebraic relations could still have the code that generated a schema from a Petri net presentation of an SMC, but the creation and execution of Workflows would go in the new package. Then when you use both Workflows and Relations, you would get checkpointing into SQL for free.

@epatters
Copy link
Member

Closing this PR, as it will be better suited to new LinAlg package if we ever pick it up again.

@epatters epatters closed this Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants